Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable solr config even when Defaults is not present. #258

Merged
merged 1 commit into from
Nov 1, 2022

Conversation

rosiel
Copy link
Contributor

@rosiel rosiel commented Nov 1, 2022

Ticket: Islandora-Devops/isle-dc#305

Currently the ISLE-DC build does not configure solr when the Islandora Starter Site is used. I tracked it down to this function, configure_islandora_default_module.

The only thing that the function did was to set the Solr URL and port.

I first just removed the whole if statement, but then decided it would be prudent to at least make sure search_api is enabled.

I did not rename the function because I am not sure how to manage the integration between versions of Buildkit and ISLE-DC. Should we do that?

Tagging @aOelschlager

@nigelgbanks
Copy link
Contributor

nigelgbanks commented Nov 1, 2022

Looks good to me.

I did not rename the function because I am not sure how to manage the integration between versions of Buildkit and ISLE-DC. Should we do that?

I think you made the right choice, we do not know who out there in the world might be calling this function, so to prevent breaking users code the first step would be to deprecated it and rename/remove it on another release.

That being said, I'm not a big fan of these utilities scripts but instead prefer folks used composers append feature to add things to settings.php like how sandbox does it:

  • Reads variables from the environment to set Drupal configuration here
  • Composer appends this file here when installed.

I'll raise a ticket to deprecate and remove the utility scripts and add documentation on how users can use the environment variables to configure their images.

@nigelgbanks nigelgbanks merged commit 4be2994 into main Nov 1, 2022
@nigelgbanks nigelgbanks deleted the configure-solr-starter-site branch November 1, 2022 17:37
@nigelgbanks
Copy link
Contributor

I've raised #260 to deal with the future deprecations / documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants